Reviewed 1 of 4 files at r1.
Reviewable status: 1 of 5 files reviewed, 1 unresolved discussion (waiting on @Pururun)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/communication/CustomListSuccess.kt line 36 at r2 (raw file):

    val name: CustomListName,
    val locationNamesAdded: List<String>? = null,
    val locationNamesRemoved: List<String>? = null,

Doesn't make sense to have nullables.

Also should we have LocationsRemoved & LocationsAdded? Or if we add the full new set of locations we could get the added/removed by doing set operations.

Reviewed 3 of 7 files at r3, 21 of 21 files at r5, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @Pururun)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/communication/CustomListActionResultData.kt line 8 at r5 (raw file):

import net.mullvad.mullvadvpn.lib.model.CustomListName

sealed interface CustomListActionResultData : Parcelable {

Introduce "Success" to split Error and Success in the result type.

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 849 at r5 (raw file):

            if (locationNames.size == 1) {
                context.getString(
                    R.string.location_was_added_to_list,

Should we have a separate string? Now it will say the same thing as if you added a item to a list.
E.g
Added 'Stockholm' to 'my_list' doesn't really say list got created.
maybe should say along the lines of:
Created list 'my_list' with location 'Stockholm'

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt line 95 at r5 (raw file):

                        { result ->
                            val resultData =
                                if (navArgs.newList) {

Maybe break out this logic into another function and return the CustomListActionResult?

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt line 108 at r5 (raw file):

                                                customListName = result.name,
                                                relayListRepository
                                                    .find(result.removedLocations.first())!!

Could be worth looking into if we should use either effect here. Then we can propagate the GenericError thingy.

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt line 242 at r5 (raw file):

        CustomListLocationsSideEffect

    data object Error : CustomListLocationsSideEffect

Not sure if we have other errors that occur or if it is enough with the generic error we already have for CustomListActionResultData.

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt line 212 at r5 (raw file):

    // data class LocationAddedToCustomList(val result: LocationsChanged) : SelectLocationSideEffect

    // class LocationRemovedFromCustomList(val result: LocationsChanged) : SelectLocationSideEffect

We should remove these if they are not to be used.

Reviewable status: 16 of 24 files reviewed, 6 unresolved discussions (waiting on @Rawa)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/communication/CustomListActionResultData.kt line 8 at r5 (raw file):

Done.

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 849 at r5 (raw file):

I agree, however I am not sure that I want to change existing copy with this PR. Maybe create a linear issue for this?

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt line 242 at r5 (raw file):

The difference is that one navigate back to the SelectLocationScreen and the other one does not and only show a toast on this screen. But I don't think they need to be different though, so will remove this.

Reviewable status: 7 of 24 files reviewed, 6 unresolved discussions (waiting on @Rawa)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt line 95 at r5 (raw file):

Done.

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt line 108 at r5 (raw file):

Done.

Reviewable status: 7 of 24 files reviewed, 6 unresolved discussions (waiting on @Rawa)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt line 212 at r5 (raw file):

Done.

Reviewed 1 of 8 files at r6, 10 of 14 files at r7, 8 of 8 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 849 at r5 (raw file):

Sounds good to me

Reviewable status: complete! all files reviewed, all discussions resolved